-
-
Notifications
You must be signed in to change notification settings - Fork 446
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[1.x] Fix useForm re-renders by memoizing functions in React #1607
Conversation
…ke setData, clearErrors, etc on every re-render
I just want to flag that this PR (mostly) fixed some issues that we were running into. I have ended up taking a copy of this I also needed to add
|
Hi @jameshulse, thank you for the hint, I've fixed it in the latest commit by using the Furthermore, using function GalleryForm(props) {
const { data, submit, errors, setData, processing, transform } = useForm({
picture: null,
picurl: gallery?.type === 0 ? gallery?.url : "",
});
const [pictureSourceType, setPictureSourceType] = useState(
data.picurl ? "pic_url" : "pic_upload"
);
transform((prevData) => ({
...prevData,
picture: prevData.type === 0 || pictureSourceType === "pic_url" ? null : prevData.picture,
picurl: prevData.type === 1 || pictureSourceType === "pic_upload" ? null : prevData.picurl,
}));
function handelSubmit(e) {
e.preventDefault();
submit("post", "gallery", {
forceFormData: true,
});
}
return (
<form>
{/* ... */}
<TabButton
onClick={() => {
setPictureSourceType("pic_upload");
}}
>
upload image
</TabButton>
<TabButton
onClick={() => {
setPictureSourceType("pic_url");
}}
>
use image url
</TabButton>
{/* ... */}
</form>
);
} Here when I update the function GalleryForm(props) {
// ...
return (
<form>
{/* ... */}
<TabButton
onClick={() => {
setPictureSourceType("pic_upload");
+ setData((prevData) => ({ ...prevData }));
}}
>
upload image
</TabButton>
<TabButton
onClick={() => {
setPictureSourceType("pic_url");
+ setData((prevData) => ({ ...prevData }));
}}
>
use image url
</TabButton>
{/* ... */}
</form>
);
} |
Anyone has an idea on what this PR is waiting? Or could I patch package this somehow? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes seem good & safe. It requires a bit of diligence to keep the dependency arrays up-to-date on the useCallback
calls, but I think worthwhile for performance.
@reinink It has been 431 days since the last commit!😶 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, thank you! As @derrickreimer mentioned, we just have to take care to maintain the dependency arrays.
@joetannenbaum |
* master: (95 commits) [1.x] Fix props reactivity (inertiajs#1969) [1.x] useForm wrongly overwrites default values after successful submission (inertiajs#1935) Update changelog [1.x] Fix `resetScrollPositions` and `restoreScrollPositions` router methods (inertiajs#1980) [1.x] Fix [scroll-region] on html element with overflow-scroll (inertiajs#1782) [1.x] Fix useForm re-renders by memoizing functions in React (inertiajs#1607) [1.x] Fix "DataCloneError: <Object> could not be cloned" (inertiajs#1967) [1.x] preserveScroll should be true on initial page visit (inertiajs#1360) Fix type augmentation (inertiajs#1958) [1.x] Fix doubling hash in React StrictMode (inertiajs#1728) [1.x] Add SSR support for Svelte 5 (inertiajs#1970) [1.x] Fix <Render /> component to respect "preserveState" (inertiajs#1943) [1.x] Fix 'received an unexpected slot "default"' warning (inertiajs#1941) QA: Add @types/lodash to fix svelte-check QA: Update reactive if statement Review useForm types QA: Move the if server up QA: Revert tsconfig change QA: Remove plural QA: Remove unused props type + add extra types just in case ... # Conflicts: # packages/react/src/index.ts
When using the
useForm()
in a React component that has some heavy pure components(components usingReact.memo
) as children and passing some methods returned byuseForm()
likesetData
,clearErrors
, etc to them, on every re-render, these methods returned byuseForm()
are getting new references so new props are passed to theseReact.memo()
ed children and they will be re-rendered unnecessarily.This pull request fixes this issue by using
useCallback
for those methods in theuseForm
hook.